-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
21367: [Spike] Nightly job to filter and sync contacts #241
21367: [Spike] Nightly job to filter and sync contacts #241
Conversation
65c65d1
to
36ff8f1
Compare
src/main/java/com/impactupgrade/nucleus/controller/AccountingController.java
Outdated
Show resolved
Hide resolved
src/main/java/com/impactupgrade/nucleus/controller/AccountingController.java
Outdated
Show resolved
Hide resolved
src/main/java/com/impactupgrade/nucleus/controller/PaymentGatewayController.java
Outdated
Show resolved
Hide resolved
src/main/java/com/impactupgrade/nucleus/service/segment/SynchronizationService.java
Outdated
Show resolved
Hide resolved
30a8db5
to
7bcf057
Compare
ea1d1f8
to
75ef24c
Compare
src/main/java/com/impactupgrade/nucleus/service/segment/AccountingPlatformService.java
Outdated
Show resolved
Hide resolved
src/main/java/com/impactupgrade/nucleus/service/segment/XeroAccountingPlatformService.java
Outdated
Show resolved
Hide resolved
src/main/java/com/impactupgrade/nucleus/service/segment/XeroAccountingPlatformService.java
Outdated
Show resolved
Hide resolved
src/main/java/com/impactupgrade/nucleus/service/segment/XeroDataSyncService.java
Outdated
Show resolved
Hide resolved
src/main/java/com/impactupgrade/nucleus/service/segment/XeroDataSyncService.java
Outdated
Show resolved
Hide resolved
src/main/java/com/impactupgrade/nucleus/service/segment/XeroAccountingPlatformService.java
Outdated
Show resolved
Hide resolved
src/main/java/com/impactupgrade/nucleus/service/segment/XeroAccountingPlatformService.java
Outdated
Show resolved
Hide resolved
@@ -25,6 +25,9 @@ public class AccountingTransaction { | |||
public String paymentGatewayName; | |||
public String paymentGatewayTransactionId; | |||
public Boolean recurring; | |||
//TODO: find a way of passing crm object custom fields to accounting layer | |||
// (configurable list of custom fields' names? extract into a map to use instead of entire crmDonation object?) | |||
public CrmDonation crmDonation; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@VSydor Can you clarify why this is needed? Are there additional fields that we need to save in Xero itself?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh nm, I see it. updateOrCreateTransactions
only has access to the accountingTransactions, right? Do we need to refactor a bit? Have this method receive the List<CrmContact>
directly, and then we move the translation into List<AccountingTransaction>
here instead of upstream in XeroDataSyncService?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Decided to add crmdonation here bcz need to check custom fields in the XeroAccountingPlatformService (WPG__c, Tax_Deductible_Gift__c, Other_Income__c).
Can add List directly to updateOrCreateTransactions method and then do translation inside XeroAccountingPlatformService for sure.
env.logJobError("Failed to upsert invoices! {}", getExceptionDetails(e)); | ||
throw e; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@VSydor Don't we need a dr-hub PR to override this? dr-hub overrode the original createTransaction
to turn TICKET transactions into a Bank Transaction (skipping the Invoice).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you're right. Will add the needed changes in dr-hub.
return queryResults; | ||
} | ||
|
||
protected QueryResult queryDonorContacts(String updatedSinceClause, String... extraFields) throws ConnectionException, InterruptedException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@VSydor One thing I realized we're missing in this: querying for business accounts. Check out AccountingService.getDonationContact. Similar to what we're doing for the MC sync, if the donation only had an Account associated with it and not a Contact, we might still need to create a faux CrmContact. Otherwise, that organization will not have created a Xero Contact by the time we sync its donations.
Ex: https://destinyrescue-fnsf.lightning.force.com/lightning/r/Opportunity/006J3000003wRMNIA2/view (business gift, no contact on the gift, no primary contact defined on the account).
So we might have to introduce that faux contact concept for this as well (further upstream, though)? Query for all non-household Accounts that are donors and updated since the date, create faux CrmContacts for them, and sync those to Xero as well?
Actually, I partially take that back. Since they want the "ContactPerson" included, we have to get a contact in addition to the business? Query for non-household accounts, get their primary contacts, and then return those as CrmContacts with the CrmAccount included inside it?
Not all Accounts have primary contacts defined on them in SFDC. Many do, but it's optional. In the doc, I have a convo going in the doc with Maria on what to do in this case. Most of them WILL at least have an affiliation defined that we could use to get a Contact...
src/main/java/com/impactupgrade/nucleus/service/segment/XeroAccountingPlatformService.java
Outdated
Show resolved
Hide resolved
src/main/java/com/impactupgrade/nucleus/service/segment/XeroAccountingPlatformService.java
Outdated
Show resolved
Hide resolved
src/main/java/com/impactupgrade/nucleus/service/segment/XeroAccountingPlatformService.java
Outdated
Show resolved
Hide resolved
if ("true".equalsIgnoreCase(getCustomDonationField(accountingTransaction, WPG_FIELD_NAME))) { | ||
lineItem.setAccountCode("120"); | ||
lineItem.setItemCode("RecurringWPG"); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just CC'd you on a question to Maria on https://docs.google.com/document/d/1wEjPtMvVwZr-k3lFa5D6M9NgP6IGZn_v/edit. This code would be creating a single invoice coded to 120 if a recurring gift had WPG=true in SFDC. Which MIGHT be correct. But as I reread what they had in their mappings, wondering if what they want is BOTH an invoice coded to 122 AND a "receive money" coded to 120.
IIRC, they want the latter, so we'd need to break this up to create both an invoice and a receive money with their independent line items. That's currently done by dr-hub's overridden createTransaction
, but we could get rid of that and just do it here...
af8b293
to
612c5b5
Compare
0b06456
to
af37dcb
Compare
b037855
to
d1cc80f
Compare
…SFDC to Xero/Stripe; Code review comments
… mappings for household/org account types
…l address defined
…d AccountingTransaction and replaced with direct usage of CrmDonation/CrmContact/CrmAccount, deleting dead code, attempting to post invoices using contact accountNumber instead of UUID
…n't work), paging the getInvoices/getTransactions calls, more simplifications/refactors
…eplaced by the new processor pattern
b596018
to
7ea980f
Compare
No description provided.